Skip to content

Comments

Fix hyphen html comment syntax highlight overflow#116

Open
wmcmurray wants to merge 2 commits intohedefalk:masterfrom
wmcmurray:patch-1
Open

Fix hyphen html comment syntax highlight overflow#116
wmcmurray wants to merge 2 commits intohedefalk:masterfrom
wmcmurray:patch-1

Conversation

@wmcmurray
Copy link

@wmcmurray wmcmurray commented Oct 21, 2022

Currently, when an html comment like this one is used: <!---------- Sample Comment -----------> (no spaces between the hyphens forming the line and those forming the comment delimiter), everything under it will be grayed.

I looked at HTML's comments RFC specs to verify if a comment like this is illegal or not, but it seems good to me, so I copied the same value used in language-html to make sure it's consistent, and it fixed the issue.

Also fun fact, Github's syntax highlight also seem to suffer this same problem ! 🤔 🤯 !

Here is an HTML syntax highlight: 👍🏼

<template lang="html">
  <!---------- Sample Comment ----------->
  <p>blablabla</p>
</template>

<script>
export default {}
</script>

and here is a Vue syntax highlight: 👎🏼

<template lang="html">
  <!---------- Sample Comment ----------->
  <p>blablabla</p>
</template>

<script>
export default {}
</script>

@confused-Techie
Copy link
Collaborator

confused-Techie commented Aug 3, 2023

While from my personal understanding of the spec you linked would say be Each comment starts with --' and includes all text up to and including the next occurrence of --'. meaning that <!---- in this snippet the second set of -- would determine the end of the comment, since it's the next occurrence, if we look elsewhere in Atom/Pulsar, where the TextMate HTML grammar accepts your entire example comment as a comment, as well as the TreeSitter HTML grammar, I'm willing to side with the other grammars in Pulsar/Atom and assume this must be valid and my interpretation is instead wrong.

Although, realistically, HTML is a very non-strict standard, where most browsers make a best effort to accept all HTML, even if invalid. Resulting in many pages online being technically invalid, but still achieving the results intended.

With all of that said, thanks a ton for contributing! I know it's taken some time to getting this PR reviewed, and I've since pushed some updates that do automatic testing, to see how this PR behaves. But I'll go ahead and as soon as I can provide a review on the content of your code as well. Appreciate your time!

@confused-Techie
Copy link
Collaborator

confused-Techie commented Aug 3, 2023

So was able to test this out, and seems your PR isn't behaving the way it's intended to.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wmcmurray @confused-Techie